feat: support logs-agent resources configuration #470

fidel-ibm · 2025-03-05T15:58:30Z

Description

Allows configuration of resources for the logs-agent.

Git issue: #469

Release required?

No release
Patch release (x.x.X)
Minor release (x.X.x)
Major release (X.x.x)

Release notes content

Run the pipeline

If the CI pipeline doesn't run when you create the PR, the PR requires a user with GitHub collaborators access to run the pipeline.

Run the CI pipeline when the PR is ready for review and you expect tests to pass. Add a comment to the PR with the following text:

/run pipeline

Checklist for reviewers

If relevant, a test for the change is included or updated with this PR.
If relevant, documentation for the change is included or updated with this PR.

For mergers

Use a conventional commit message to set the release level. Follow the guidelines.
Include information that users need to know about the PR in the commit message. The commit message becomes part of the GitHub release notes.
Use the Squash and merge option.

ocofaigh

Just 1 comment from me. @Aashiq-J as codeowner you any other concerns?

variables.tf

shemau

This looks good to me.

modules/logs-agent/variables.tf

shemau · 2025-03-05T20:01:42Z

/run pipeline

modules/logs-agent/main.tf

ocofaigh · 2025-03-06T14:14:21Z

/run pipeline

ocofaigh · 2025-03-06T14:22:49Z

Oh the tests are blocked due to this issue: IBM-Cloud/terraform-provider-ibm#6036
There is supposed to be a provider release today with the fix, let me follow up on that

ocofaigh · 2025-03-07T10:14:30Z

Provider patch release coming today - will re-run after its released

ocofaigh · 2025-03-07T10:59:08Z

/run pipeline

ocofaigh · 2025-03-07T14:13:25Z

/run pipeline

ocofaigh

@fidel-ibm Your going to have to do some testing to get this to work. The tests failed with:

│ Error: failed parsing key "resources" with value {"limits":{"cpu":"500m","ephemeral_storage":"10Gi","memory":"3Gi"},"requests":{"cpu":"100m","ephemeral_storage":"2Gi","memory":"1Gi"}}, key "\"requests\":{\"cpu\":\"100m\"" has no value (cannot end with ,)
│ 
│   with module.observability_agents.module.logs_agent[0].helm_release.logs_agent,
│   on ../../modules/logs-agent/main.tf line 40, in resource "helm_release" "logs_agent":
│   40: resource "helm_release" "logs_agent" {
│ 
╵}

ocofaigh · 2025-03-08T11:02:07Z

modules/logs-agent/main.tf


+  set {
+    name  = "resources"
+    value = jsonencode(var.logs_agent_resources)


maybe you need to use join("\\," like what was done in other sets for object types

hmm not sure what the issue is in this case, I cannot use join because that expect a list and we are passing an object.

I've moved this code to inside the values definition, to see if the yamlencode function handles the transformation well.

When I run terraform plan even with the previous setup, it had the proper output for the config.

modules/logs-agent/variables.tf

variables.tf

ocofaigh

@fidel-ibm see comments

…rces

ocofaigh · 2025-03-10T15:53:41Z

/run pipeline

…thub.com:fidel-ibm/terraform-ibm-observability-agents into fidel-ruiz/feat-add-support-logs-agent-resources

…rces

ocofaigh · 2025-03-13T10:00:29Z

/run pipeline

ocofaigh · 2025-03-14T09:24:12Z

@fidel-ibm Any joy locally testing this? The tests are failing with:

TestRunAgentVpcKubernetes 2025-03-13T10:37:31Z logger.go:67: │ Error: 1 error occurred:
TestRunAgentVpcKubernetes 2025-03-13T10:37:31Z logger.go:67: │ 	* DaemonSet.apps "logs-agent" is invalid: [spec.template.spec.containers[0].resources.limits[ephemeral_storage]: Invalid value: ephemeral_storage: must be a standard resource type or fully qualified, spec.template.spec.containers[0].resources.limits[ephemeral_storage]: Invalid value: ephemeral_storage: must be a standard resource for containers, spec.template.spec.containers[0].resources.requests[ephemeral_storage]: Invalid value: ephemeral_storage: must be a standard resource type or fully qualified, spec.template.spec.containers[0].resources.requests[ephemeral_storage]: Invalid value: ephemeral_storage: must be a standard resource for containers]

jor2 · 2025-03-25T14:30:56Z

I see ephemeral_storage is included in the doc, but I can't see any evidence kubernetes supports ephemeral_storage as a standard resource type for containers.

ocofaigh · 2025-03-26T16:37:12Z

@jor2 I see you added support in terraform-ibm-modules/terraform-ibm-logs-agent#2 - Does it mean you got it working? Can you leave a comment in this PR as to what the issue is?

jor2 · 2025-03-26T18:36:45Z

@jor2 I see you added support in terraform-ibm-modules/terraform-ibm-logs-agent#2 - Does it mean you got it working? Can you leave a comment in this PR as to what the issue is?

It works when you remove the field ephemeral_storage, doesn't seem like its a supported field yet IBM cloud docs suggests it is. I simply removed that field from other pr #470 (comment)

ocofaigh mentioned this pull request Mar 5, 2025

[ terraform-ibm-observability-agents/modules/logs-agent] Include variables to configure agent resources #469

Open

ocofaigh requested changes Mar 5, 2025

View reviewed changes

variables.tf Show resolved Hide resolved

fidel-ibm force-pushed the fidel-ruiz/feat-add-support-logs-agent-resources branch from 18540f5 to 84309df Compare March 5, 2025 16:10

fidel-ibm marked this pull request as ready for review March 5, 2025 16:28

fidel-ibm requested review from Aashiq-J and shemau as code owners March 5, 2025 16:28

shemau requested changes Mar 5, 2025

View reviewed changes

modules/logs-agent/variables.tf Outdated Show resolved Hide resolved

fidel-ibm force-pushed the fidel-ruiz/feat-add-support-logs-agent-resources branch from 84309df to 2d78c4c Compare March 5, 2025 19:56

ocofaigh reviewed Mar 6, 2025

View reviewed changes

modules/logs-agent/main.tf Outdated Show resolved Hide resolved

fidel-ibm force-pushed the fidel-ruiz/feat-add-support-logs-agent-resources branch 2 times, most recently from a8a6f02 to 85a4517 Compare March 6, 2025 14:08

ocofaigh requested changes Mar 8, 2025

View reviewed changes

ocofaigh reviewed Mar 8, 2025

View reviewed changes

modules/logs-agent/variables.tf Outdated Show resolved Hide resolved

ocofaigh reviewed Mar 8, 2025

View reviewed changes

variables.tf Outdated Show resolved Hide resolved

ocofaigh requested changes Mar 8, 2025

View reviewed changes

fidel-ibm added 2 commits March 10, 2025 10:05

feat: support logs-agent resources configuration

762aafb

feat: support logs-agent resources configuration

43c3b7d

fidel-ibm force-pushed the fidel-ruiz/feat-add-support-logs-agent-resources branch from d5eddd8 to 6968fac Compare March 10, 2025 14:20

address failing pre-commit hook

7b0c654

fidel-ibm force-pushed the fidel-ruiz/feat-add-support-logs-agent-resources branch from 6968fac to 7b0c654 Compare March 10, 2025 14:24

ocofaigh and others added 3 commits March 10, 2025 14:24

Merge branch 'main' into fidel-ruiz/feat-add-support-logs-agent-resou…

c4a64bb

…rces

Merge remote-tracking branch 'upstream/main'

4d4a4b9

Merge branch 'main' into fidel-ruiz/feat-add-support-logs-agent-resou…

8e8ee95

…rces

fidel-ibm force-pushed the fidel-ruiz/feat-add-support-logs-agent-resources branch from a819b41 to 43eafd5 Compare March 10, 2025 15:43

Merge branch 'fidel-ruiz/feat-add-support-logs-agent-resources' of gi…

3a47f2f

…thub.com:fidel-ibm/terraform-ibm-observability-agents into fidel-ruiz/feat-add-support-logs-agent-resources

fidel-ibm force-pushed the fidel-ruiz/feat-add-support-logs-agent-resources branch from 43eafd5 to 3a47f2f Compare March 10, 2025 17:25

Merge branch 'main' into fidel-ruiz/feat-add-support-logs-agent-resou…

16fd472

…rces

ocofaigh mentioned this pull request Mar 25, 2025

feat: move logs agents to its own tile terraform-ibm-modules/terraform-ibm-logs-agent#2

Merged

6 tasks

feat: support logs-agent resources configuration #470

Are you sure you want to change the base?

feat: support logs-agent resources configuration #470

Conversation

fidel-ibm commented Mar 5, 2025

Description

Release required?

Release notes content

Run the pipeline

Checklist for reviewers

For mergers

Uh oh!

ocofaigh left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

shemau left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

shemau commented Mar 5, 2025

Uh oh!

Uh oh!

ocofaigh commented Mar 6, 2025

Uh oh!

ocofaigh commented Mar 6, 2025

Uh oh!

ocofaigh commented Mar 7, 2025

Uh oh!

ocofaigh commented Mar 7, 2025

Uh oh!

ocofaigh commented Mar 7, 2025

Uh oh!

ocofaigh left a comment

Choose a reason for hiding this comment

Uh oh!

ocofaigh Mar 8, 2025

Choose a reason for hiding this comment

Uh oh!

fidel-ibm Mar 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

ocofaigh left a comment

Choose a reason for hiding this comment

Uh oh!

ocofaigh commented Mar 10, 2025

Uh oh!

ocofaigh commented Mar 13, 2025

Uh oh!

ocofaigh commented Mar 14, 2025

Uh oh!

jor2 commented Mar 25, 2025

Uh oh!

ocofaigh commented Mar 26, 2025

Uh oh!

jor2 commented Mar 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

fidel-ibm Mar 10, 2025 •

edited

Loading

jor2 commented Mar 26, 2025 •

edited

Loading